From 455f800cb9db414648bf3d84ae6441bf41665c50 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 18 Feb 2016 23:51:52 -0800 Subject: [PATCH] Refactor configuration return values Wrap up the value/definition pair in a generic structure so we can extend it well later. --- src/cargo/ops/cargo_compile.rs | 28 +++++---- src/cargo/ops/cargo_install.rs | 6 +- src/cargo/ops/cargo_new.rs | 108 ++++++++++++++++----------------- src/cargo/ops/registry.rs | 8 +-- src/cargo/util/config.rs | 93 ++++++++++++++++++++-------- 5 files changed, 146 insertions(+), 97 deletions(-) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index ba7716f12..5226a8aee 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -421,19 +421,21 @@ fn scrape_build_config(config: &Config, target: Option) -> CargoResult { let cfg_jobs = match try!(config.get_i64("build.jobs")) { - Some((n, p)) => { - if n <= 0 { - bail!("build.jobs must be positive, but found {} in {:?}", n, p) - } else if n >= u32::max_value() as i64 { - bail!("build.jobs is too large: found {} in {:?}", n, p) + Some(v) => { + if v.val <= 0 { + bail!("build.jobs must be positive, but found {} in {}", + v.val, v.definition) + } else if v.val >= u32::max_value() as i64 { + bail!("build.jobs is too large: found {} in {}", v.val, + v.definition) } else { - Some(n as u32) + Some(v.val as u32) } } None => None, }; let jobs = jobs.or(cfg_jobs).unwrap_or(::num_cpus::get() as u32); - let cfg_target = try!(config.get_string("build.target")).map(|s| s.0); + let cfg_target = try!(config.get_string("build.target")).map(|s| s.val); let target = target.or(cfg_target); let mut base = ops::BuildConfig { jobs: jobs, @@ -453,16 +455,18 @@ fn scrape_target_config(config: &Config, triple: &str) let key = format!("target.{}", triple); let mut ret = ops::TargetConfig { - ar: try!(config.get_path(&format!("{}.ar", key))), - linker: try!(config.get_path(&format!("{}.linker", key))), + ar: try!(config.get_path(&format!("{}.ar", key))).map(|v| v.val), + linker: try!(config.get_path(&format!("{}.linker", key))).map(|v| v.val), overrides: HashMap::new(), }; let table = match try!(config.get_table(&key)) { - Some((table, _)) => table, + Some(table) => table.val, None => return Ok(ret), }; for (lib_name, _) in table.into_iter() { - if lib_name == "ar" || lib_name == "linker" { continue } + if lib_name == "ar" || lib_name == "linker" { + continue + } let mut output = BuildOutput { library_paths: Vec::new(), @@ -472,7 +476,7 @@ fn scrape_target_config(config: &Config, triple: &str) rerun_if_changed: Vec::new(), }; let key = format!("{}.{}", key, lib_name); - let table = try!(config.get_table(&key)).unwrap().0; + let table = try!(config.get_table(&key)).unwrap().val; for (k, _) in table.into_iter() { let key = format!("{}.{}", key, k); match try!(config.get(&key)).unwrap() { diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 36fdabaf4..95eb4238e 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -340,11 +340,11 @@ pub fn uninstall(root: Option<&str>, } fn resolve_root(flag: Option<&str>, config: &Config) -> CargoResult { - let config_root = try!(config.get_string("install.root")); + let config_root = try!(config.get_path("install.root")); Ok(flag.map(PathBuf::from).or_else(|| { env::var_os("CARGO_INSTALL_ROOT").map(PathBuf::from) - }).or_else(|| { - config_root.clone().map(|(v, _)| PathBuf::from(v)) + }).or_else(move || { + config_root.map(|v| v.val) }).unwrap_or_else(|| { config.home().to_owned() })) diff --git a/src/cargo/ops/cargo_new.rs b/src/cargo/ops/cargo_new.rs index 309c75c61..41fa01fc1 100644 --- a/src/cargo/ops/cargo_new.rs +++ b/src/cargo/ops/cargo_new.rs @@ -63,17 +63,17 @@ fn get_name<'a>(path: &'a Path, opts: &'a NewOptions, config: &Config) -> CargoR if let Some(name) = opts.name { return Ok(name); } - + if path.file_name().is_none() { bail!("cannot auto-detect project name from path {:?} ; use --name to override", path.as_os_str()); } - + let dir_name = try!(path.file_name().and_then(|s| s.to_str()).chain_error(|| { human(&format!("cannot create a project with a non-unicode name: {:?}", path.file_name().unwrap())) })); - + if opts.bin { Ok(dir_name) } else { @@ -99,24 +99,24 @@ fn check_name(name: &str) -> CargoResult<()> { Ok(()) } -fn detect_source_paths_and_types(project_path : &Path, - project_name: &str, +fn detect_source_paths_and_types(project_path : &Path, + project_name: &str, detected_files: &mut Vec, ) -> CargoResult<()> { let path = project_path; let name = project_name; - + enum H { Bin, Lib, Detect, } - + struct Test { proposed_path: String, handling: H, } - + let tests = vec![ Test { proposed_path: format!("src/main.rs"), handling: H::Bin }, Test { proposed_path: format!("main.rs"), handling: H::Bin }, @@ -125,48 +125,48 @@ fn detect_source_paths_and_types(project_path : &Path, Test { proposed_path: format!("src/lib.rs"), handling: H::Lib }, Test { proposed_path: format!("lib.rs"), handling: H::Lib }, ]; - + for i in tests { let pp = i.proposed_path; - + // path/pp does not exist or is not a file if !fs::metadata(&path.join(&pp)).map(|x| x.is_file()).unwrap_or(false) { continue; } - + let sfi = match i.handling { H::Bin => { - SourceFileInformation { - relative_path: pp, - target_name: project_name.to_string(), - bin: true + SourceFileInformation { + relative_path: pp, + target_name: project_name.to_string(), + bin: true } } H::Lib => { - SourceFileInformation { - relative_path: pp, - target_name: project_name.to_string(), + SourceFileInformation { + relative_path: pp, + target_name: project_name.to_string(), bin: false } } H::Detect => { let content = try!(paths::read(&path.join(pp.clone()))); let isbin = content.contains("fn main"); - SourceFileInformation { - relative_path: pp, - target_name: project_name.to_string(), - bin: isbin + SourceFileInformation { + relative_path: pp, + target_name: project_name.to_string(), + bin: isbin } } }; detected_files.push(sfi); } - + // Check for duplicate lib attempt - + let mut previous_lib_relpath : Option<&str> = None; let mut duplicates_checker : BTreeMap<&str, &SourceFileInformation> = BTreeMap::new(); - + for i in detected_files { if i.bin { if let Some(x) = BTreeMap::get::(&duplicates_checker, i.target_name.as_ref()) { @@ -188,13 +188,13 @@ cannot automatically generate Cargo.toml as the main target would be ambiguous", previous_lib_relpath = Some(&i.relative_path); } } - + Ok(()) } fn plan_new_source_file(bin: bool, project_name: String) -> SourceFileInformation { if bin { - SourceFileInformation { + SourceFileInformation { relative_path: "src/main.rs".to_string(), target_name: project_name, bin: true, @@ -214,7 +214,7 @@ pub fn new(opts: NewOptions, config: &Config) -> CargoResult<()> { bail!("destination `{}` already exists", path.display()) } - + let name = try!(get_name(&path, &opts, config)); try!(check_name(name)); @@ -225,7 +225,7 @@ pub fn new(opts: NewOptions, config: &Config) -> CargoResult<()> { source_files: vec![plan_new_source_file(opts.bin, name.to_string())], bin: opts.bin, }; - + mk(config, &mkopts).chain_error(|| { human(format!("Failed to create project `{}` at `{}`", name, path.display())) @@ -234,19 +234,19 @@ pub fn new(opts: NewOptions, config: &Config) -> CargoResult<()> { pub fn init(opts: NewOptions, config: &Config) -> CargoResult<()> { let path = config.cwd().join(opts.path); - + let cargotoml_path = path.join("Cargo.toml"); if fs::metadata(&cargotoml_path).is_ok() { bail!("`cargo init` cannot be run on existing Cargo projects") } - + let name = try!(get_name(&path, &opts, config)); try!(check_name(name)); - + let mut src_paths_types = vec![]; - + try!(detect_source_paths_and_types(&path, name, &mut src_paths_types)); - + if src_paths_types.len() == 0 { src_paths_types.push(plan_new_source_file(opts.bin, name.to_string())); } else { @@ -254,24 +254,24 @@ pub fn init(opts: NewOptions, config: &Config) -> CargoResult<()> { // Maybe when doing `cargo init --bin` inside a library project stub, // user may mean "initialize for library, but also add binary target" } - + let mut version_control = opts.version_control; - + if version_control == None { let mut num_detected_vsces = 0; - + if fs::metadata(&path.join(".git")).is_ok() { version_control = Some(VersionControl::Git); num_detected_vsces += 1; } - + if fs::metadata(&path.join(".hg")).is_ok() { version_control = Some(VersionControl::Hg); num_detected_vsces += 1; } - + // if none exists, maybe create git, like in `cargo new` - + if num_detected_vsces > 1 { bail!("both .git and .hg directories found \ and the ignore file can't be \ @@ -279,7 +279,7 @@ pub fn init(opts: NewOptions, config: &Config) -> CargoResult<()> { specify --vcs to override detection"); } } - + let mkopts = MkOptions { version_control: version_control, path: &path, @@ -287,7 +287,7 @@ pub fn init(opts: NewOptions, config: &Config) -> CargoResult<()> { bin: src_paths_types.iter().any(|x|x.bin), source_files: src_paths_types, }; - + mk(config, &mkopts).chain_error(|| { human(format!("Failed to create project `{}` at `{}`", name, path.display())) @@ -357,11 +357,11 @@ fn mk(config: &Config, opts: &MkOptions) -> CargoResult<()> { (Some(name), None, _, None) | (None, None, name, None) => name, }; - + let mut cargotoml_path_specifier = String::new(); - + // Calculare what [lib] and [[bin]]s do we need to append to Cargo.toml - + for i in &opts.source_files { if i.bin { if i.relative_path != "src/main.rs" { @@ -394,17 +394,17 @@ authors = [{}] {}"#, name, toml::Value::String(author), cargotoml_path_specifier).as_bytes())); - // Create all specified source files - // (with respective parent directories) + // Create all specified source files + // (with respective parent directories) // if they are don't exist for i in &opts.source_files { let path_of_source_file = path.join(i.relative_path.clone()); - + if let Some(src_dir) = path_of_source_file.parent() { try!(fs::create_dir_all(src_dir)); } - + let default_file_content : &[u8] = if i.bin { b"\ fn main() { @@ -421,7 +421,7 @@ mod tests { } " }; - + if !fs::metadata(&path_of_source_file).map(|x| x.is_file()).unwrap_or(false) { return paths::write(&path_of_source_file, default_file_content) } @@ -454,18 +454,18 @@ fn discover_author() -> CargoResult<(String, Option)> { } fn global_config(config: &Config) -> CargoResult { - let name = try!(config.get_string("cargo-new.name")).map(|s| s.0); - let email = try!(config.get_string("cargo-new.email")).map(|s| s.0); + let name = try!(config.get_string("cargo-new.name")).map(|s| s.val); + let email = try!(config.get_string("cargo-new.email")).map(|s| s.val); let vcs = try!(config.get_string("cargo-new.vcs")); - let vcs = match vcs.as_ref().map(|p| (&p.0[..], &p.1)) { + let vcs = match vcs.as_ref().map(|p| (&p.val[..], &p.definition)) { Some(("git", _)) => Some(VersionControl::Git), Some(("hg", _)) => Some(VersionControl::Hg), Some(("none", _)) => Some(VersionControl::NoVcs), Some((s, p)) => { return Err(internal(format!("invalid configuration for key \ `cargo-new.vcs`, unknown vcs `{}` \ - (found in {:?})", s, p))) + (found in {})", s, p))) } None => None }; diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 9bbdf8712..0b2f83e18 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -126,8 +126,8 @@ fn transmit(pkg: &Package, tarball: &Path, registry: &mut Registry) } pub fn registry_configuration(config: &Config) -> CargoResult { - let index = try!(config.get_string("registry.index")).map(|p| p.0); - let token = try!(config.get_string("registry.token")).map(|p| p.0); + let index = try!(config.get_string("registry.index")).map(|p| p.val); + let token = try!(config.get_string("registry.token")).map(|p| p.val); Ok(RegistryConfig { index: index, token: token }) } @@ -182,7 +182,7 @@ pub fn http_handle(config: &Config) -> CargoResult { /// via environment variables are picked up by libcurl. fn http_proxy(config: &Config) -> CargoResult> { match try!(config.get_string("http.proxy")) { - Some((s, _)) => return Ok(Some(s)), + Some(s) => return Ok(Some(s.val)), None => {} } match git2::Config::open_default() { @@ -218,7 +218,7 @@ pub fn http_proxy_exists(config: &Config) -> CargoResult { pub fn http_timeout(config: &Config) -> CargoResult> { match try!(config.get_i64("http.timeout")) { - Some((s, _)) => return Ok(Some(s)), + Some(s) => return Ok(Some(s.val)), None => {} } Ok(env::var("HTTP_TIMEOUT").ok().and_then(|s| s.parse().ok())) diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index 55c654373..5160a91f7 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -148,50 +148,74 @@ impl Config { Ok(Some(val.clone())) } - pub fn get_string(&self, key: &str) -> CargoResult> { + pub fn get_string(&self, key: &str) -> CargoResult>> { match try!(self.get(key)) { - Some(CV::String(i, path)) => Ok(Some((i, path))), + Some(CV::String(i, path)) => { + Ok(Some(Value { + val: i, + definition: Definition::Path(path), + })) + } Some(val) => self.expected("string", key, val), None => Ok(None), } } - pub fn get_path(&self, key: &str) -> CargoResult> { - if let Some((specified_path, path_to_config)) = try!(self.get_string(&key)) { - if specified_path.contains("/") || (cfg!(windows) && specified_path.contains("\\")) { - // An absolute or a relative path - let prefix_path = path_to_config.parent().unwrap().parent().unwrap(); - // Joining an absolute path to any path results in the given absolute path - Ok(Some(prefix_path.join(specified_path))) + pub fn get_path(&self, key: &str) -> CargoResult>> { + if let Some(val) = try!(self.get_string(&key)) { + let is_path = val.val.contains("/") || + (cfg!(windows) && val.val.contains("\\")); + let path = if is_path { + val.definition.root(self).join(val.val) } else { // A pathless name - Ok(Some(PathBuf::from(specified_path))) - } + PathBuf::from(val.val) + }; + Ok(Some(Value { + val: path, + definition: val.definition, + })) } else { Ok(None) } } - pub fn get_list(&self, key: &str) -> CargoResult, PathBuf)>> { + pub fn get_list(&self, key: &str) + -> CargoResult>>> { match try!(self.get(key)) { - Some(CV::List(i, path)) => Ok(Some((i, path))), + Some(CV::List(i, path)) => { + Ok(Some(Value { + val: i, + definition: Definition::Path(path), + })) + } Some(val) => self.expected("list", key, val), None => Ok(None), } } pub fn get_table(&self, key: &str) - -> CargoResult, PathBuf)>> { + -> CargoResult>>> { match try!(self.get(key)) { - Some(CV::Table(i, path)) => Ok(Some((i, path))), + Some(CV::Table(i, path)) => { + Ok(Some(Value { + val: i, + definition: Definition::Path(path), + })) + } Some(val) => self.expected("table", key, val), None => Ok(None), } } - pub fn get_i64(&self, key: &str) -> CargoResult> { + pub fn get_i64(&self, key: &str) -> CargoResult>> { match try!(self.get(key)) { - Some(CV::Integer(i, path)) => Ok(Some((i, path))), + Some(CV::Integer(i, path)) => { + Ok(Some(Value { + val: i, + definition: Definition::Path(path), + })) + } Some(val) => self.expected("integer", key, val), None => Ok(None), } @@ -244,12 +268,8 @@ impl Config { fn scrape_target_dir_config(&mut self) -> CargoResult<()> { if let Some(dir) = env::var_os("CARGO_TARGET_DIR") { *self.target_dir.borrow_mut() = Some(self.cwd.join(dir)); - } else if let Some((dir, dir2)) = try!(self.get_string("build.target-dir")) { - let mut path = PathBuf::from(dir2); - path.pop(); - path.pop(); - path.push(dir); - *self.target_dir.borrow_mut() = Some(path); + } else if let Some(val) = try!(self.get_path("build.target-dir")) { + *self.target_dir.borrow_mut() = Some(val.val); } Ok(()) } @@ -262,7 +282,7 @@ impl Config { let var = format!("build.{}", tool); if let Some(tool_path) = try!(self.get_path(&var)) { - return Ok(tool_path); + return Ok(tool_path.val); } Ok(PathBuf::from(tool)) @@ -284,6 +304,15 @@ pub enum ConfigValue { Boolean(bool, PathBuf), } +pub struct Value { + pub val: T, + pub definition: Definition, +} + +pub enum Definition { + Path(PathBuf), +} + impl fmt::Debug for ConfigValue { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match *self { @@ -466,6 +495,22 @@ impl ConfigValue { } } +impl Definition { + pub fn root<'a>(&'a self, config: &'a Config) -> &'a Path { + match *self { + Definition::Path(ref p) => p.parent().unwrap().parent().unwrap(), + } + } +} + +impl fmt::Display for Definition { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match *self { + Definition::Path(ref p) => p.display().fmt(f), + } + } +} + fn homedir(cwd: &Path) -> Option { let cargo_home = env::var_os("CARGO_HOME").map(|home| { cwd.join(home) -- 2.30.2